-
-
Notifications
You must be signed in to change notification settings - Fork 773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix xtensa formatting #1694
Fix xtensa formatting #1694
Conversation
cc6d510
to
9777555
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's one small tweak we spotted in review but otherwise this looks great. With that one item addressed, we're happy to merge this.
@@ -967,13 +967,12 @@ static void cortexr_mem_handle_fault( | |||
#if ENABLE_DEBUG == 1 | |||
const uint32_t fault_status = cortexar_coproc_read(target, CORTEXAR_DFSR); | |||
const uint32_t fault_addr = cortexar_coproc_read(target, CORTEXAR_DFAR); | |||
DEBUG_WARN("%s: Failed at 0x%08" PRIx32 " (%08" PRIx32 ")\n", func, fault_addr, fault_status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than moving this, it should be gated by #ifndef DEBUG_WARN_IS_NOOP
in place of #if ENABLE_DEBUG == 1
- this will stop the variable definitions doing the wrong thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I've updated this gate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We guess it's fine to have the DEBUG_WARN() inside the guard block, just.. not ideal as we should really complete the IO operations before reporting the failure. That said, this nominally works so we'll go with it and merge.
This function has a debug print statement that doesn't use `PRIx32`, resulting in build breakage on Farpatch. Signed-off-by: Sean Cross <[email protected]>
This function needs to use PRIu32 when printing values. This fixes the build on Farpatch. Signed-off-by: Sean Cross <[email protected]>
Using printf formatters fixes the build on ESP32 Xtensa targets. Signed-off-by: Sean Cross <[email protected]>
9777555
to
b20e346
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏼, merging. Thank you for the contribution!
@@ -967,13 +967,12 @@ static void cortexr_mem_handle_fault( | |||
#if ENABLE_DEBUG == 1 | |||
const uint32_t fault_status = cortexar_coproc_read(target, CORTEXAR_DFSR); | |||
const uint32_t fault_addr = cortexar_coproc_read(target, CORTEXAR_DFAR); | |||
DEBUG_WARN("%s: Failed at 0x%08" PRIx32 " (%08" PRIx32 ")\n", func, fault_addr, fault_status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We guess it's fine to have the DEBUG_WARN() inside the guard block, just.. not ideal as we should really complete the IO operations before reporting the failure. That said, this nominally works so we'll go with it and merge.
Detailed description
Debug print statements were added that cause build breakage on ESP32 Xtensa targets.
Your checklist for this pull request
make PROBE_HOST=native
)make PROBE_HOST=hosted
)